File musical chairs by dwalton76 · Pull Request #391 · ev3dev/ev3dev-lang-python

dwalton76 · 2017-10-03T02:31:36Z

For issue #375

@dlech @ddemidov @WasabiFan @ensonic take a look, this is a first stab at getting everything split up and moved around. TouchSensor, Motors, ColorSensor, Buttons, InfraredSensor, and LEDs are all working.

Here is what we have robot@evb[ev3dev-lang-python]# tree ev3dev/ ev3dev/ ├── button.py ├── control │ ├── GyroBalancer.py │ ├── __init__.py │ └── rc_tank.py ├── display.py ├── fonts // // chopped most of fonts to limit the output here // │ ├── timR18.pbm │ ├── timR18.pil │ ├── timR24.pbm │ └── timR24.pil ├── helper.py ├── __init__.py ├── led.py ├── motor.py ├── _platform │ ├── brickpi.py │ ├── ev3.py │ ├── evb.py │ ├── __init__.py │ └── pistorms.py ├── port.py ├── sensor │ ├── __init__.py │ └── lego.py ├── sound.py ├── version.py └── webserver.py 4 directories, 421 files robot@evb[ev3dev-lang-python]#

dwalton76 · 2017-10-03T02:33:07Z

Ignoring the fonts directory, this is the current structure

robot@evb[ev3dev]# tree .
.
├── _button.py
├── button.py
├── control
│   ├── GyroBalancer.py
│   ├── __init__.py
│   ├── rc_tank.py
│   └── webserver.py
├── display.py
├── fonts
│   ├── charB08.pbm


│   ├── timR18.pil
│   ├── timR24.pbm
│   └── timR24.pil
├── helper.py
├── __init__.py
├── _led.py
├── led.py
├── motor.py
├── _platform
│   ├── brickpi.py
│   ├── ev3.py
│   ├── evb.py
│   ├── __init__.py
│   └── pistorms.py
├── port.py
├── sensor
│   ├── __init__.py
│   └── lego.py
├── sound.py
└── version.py

4 directories, 423 files
robot@evb[ev3dev]#

dwalton76 · 2017-10-03T02:41:24Z

Can we make travis have a /sys/class/board-info/? See CI failure...

WasabiFan · 2017-10-03T02:45:17Z

Why are there two files for buttons?

dlech · 2017-10-03T02:51:39Z

version could be put in the root __init__.py, so there is no need for a module for just that.

dlech · 2017-10-03T02:52:46Z

fonts probably shouldn't be in the source code tree.

dlech · 2017-10-03T02:53:19Z

Is there anything left in helper.py after moving stuff to control?

dlech · 2017-10-03T03:02:32Z

It seems like it would be cleaner to do some import magic in _platform/__init__.py rather than having button/_button and led/_led

dwalton76 · 2017-10-03T03:02:38Z

version.py is generated via python3 setup.py install
fonts not sure what the scoop is there, I've never used them
helper.py I'll go ahead and chop it, I just need to redo the MINDCUBER demo but at this point we need to redo all of the demos so no point holding off

dwalton76 · 2017-10-03T03:05:45Z

yeah button and led need some work that was just what I came up with to get the importing working. These are both things where a platform specific class must live in _platform and that class must inherit from some parent class. So button for instance

we know we want the user to from ev3dev.button import Button
The Button class will live in _platform but needs ButtonEVIO
So where to put ButtonEVIO? It can't be in button.py else you get into a circular import situation between it and the _platform file.

dwalton76 · 2017-10-03T03:08:55Z

@ddemidov thoughts on fonts?

dlech · 2017-10-03T03:09:27Z

Why would we have platform-specific button classes? As far as I can tell, the only thing that should be different is the path to the device node.

dwalton76 · 2017-10-03T03:11:16Z

There is no guarantee that all platforms will have the same buttons...it just happens that EV3 and EVB do. And since pistorms doesn't have any it shouldn't really have a Button class.

dwalton76 · 2017-10-03T03:13:45Z

Wait I think pistorms has a back button but that is it and brickpi doesn't have any buttons.

dlech · 2017-10-03T03:15:38Z

How will documentation work if the same class is not the same between platforms? It seems to me like it would be better to make them all the same and throw a RuntimeError if something is missing (and the documentation will say something like "raises RuntimeError on BrickPi" if a class or method is not supported on that platform).

P.S. PiStorms has a Go button. (And one can add buttons to BrickPi using extra gpios)

dwalton76 · 2017-10-03T03:25:37Z

I'll give you it would certainly clean up the importing. What about LEDs though they differ a good deal from EV3 vs pistorms?

ddemidov · 2017-10-03T03:49:26Z

Out of interest, what does time python3 -c 'import ev3dev.ev3' (before the PR) and time python3 -c 'import ev3dev.motor' (after the PR) show?

ddemidov · 2017-10-03T08:40:07Z

thoughts on fonts?

I don't see why we could not leave them to be as is, except they are only usable with python3/PIL.

ddemidov · 2017-10-03T08:48:17Z

Can we make travis have a /sys/class/board-info/? See CI failure...

We could put a board-info file into ddemidov/ev3dev-lang-fake-sys (btw, should we move that repo under ev3dev?). Then we need to make the get_current_platform function to respect Device.DEVICE_ROOT_PATH constant (or, better yet, move the constant somewhere else). This way the travis tests will know where to look for the file.

ddemidov · 2017-10-03T08:53:52Z

See #225 for some background on fonts.

ddemidov · 2017-10-03T08:54:41Z

@dlech,

fonts probably shouldn't be in the source code tree

Do you mean we should put them into a separate package?

ddemidov · 2017-10-03T08:57:34Z

Re platform specific buttons: platform file could provide both path for the event file and the list of supported buttons. Since python is a dynamic language, we could use that list (during import) to create properties corresponding to the buttons inside Button class.

dwalton76 · 2017-10-03T12:06:39Z

Before

robot@ev3dev[~]# time python3 -c 'import ev3dev.ev3'

real	0m14.938s
user	0m11.340s
sys	0m0.560s
robot@ev3dev[~]#

After

robot@ev3dev[~]# time python3 -c 'import ev3dev.motor' 

real	0m11.645s
user	0m9.090s
sys	0m0.510s
robot@ev3dev[~]#

dwalton76 · 2017-10-03T12:11:46Z

Latest commit removes _button.py with the assumption that all platforms have the same buttons. I'll test later on a pistorms and have it raise some NoButton exception if the user tries to use buttons on a device that doesn't have them. Do you guys like the way this looks more? If so I'll do the same for LEDs.

dwalton76 · 2017-10-03T12:12:15Z

btw, should we move that repo under ev3dev?

yes

dwalton76 · 2017-10-03T12:13:40Z

We could put a board-info file into ddemidov/ev3dev-lang-fake-sys

cool, @ddemidov can you add that? I haven't done much with ev3dev-lang-fake-sys

ddemidov · 2017-10-03T12:29:10Z

What should be in there? Should FAKE-SYS work?

dwalton76 · 2017-10-03T12:59:13Z

Works for me

dwalton76 · 2017-10-03T13:33:10Z

Should we move 'PowerSupply' out of 'init.py' and into a 'power.py'?

WasabiFan · 2017-10-03T13:38:24Z

Should we move 'PowerSupply' out of 'init.py' and into a 'power.py'?

I'd say "yes", because it's pretty rare that someone would use the power supply class. In my mind it's just clutter for most use cases. (In fact, I think we should try to minimize the classes in init.py overall)

dlech · 2017-10-03T15:49:12Z

What about LEDs though they differ a good deal from EV3 vs pistorms?

How so? PiStorms just has one extra color. I wouldn't call that a big difference.

dlech · 2017-10-03T15:52:56Z

@dlech,
fonts probably shouldn't be in the source code tree
Do you mean we should put them into a separate package?

I'm not sure what is the usual way for distributing data files with python packages, but is just seems odd to have data files mixed in with source code files in the source repository. Maybe better to save this discussion for later though.

dlech · 2017-10-03T15:54:21Z

 INPUT_3 = 'in3'
 INPUT_4 = 'in4'

+buttons_filename = '/dev/input/by-path/platform-gpio_keys-event'


All caps for constants?

dwalton76 · 2017-10-03T15:55:52Z

I was thinking EV3 had more LEDs than pistorms but now that I look at http://www.ev3dev.org/docs/platform-comparison/ I see that it doesn't. I'll clean up LEDs to follow the same model as buttons.

dlech · 2017-10-03T15:56:37Z


+import array
+import time
+import evdev


evdev is probably a linux-specific module like fcntl

I was meaning to ask about the try/except around fcntl...do we have folks running api_test.py on windows?

Probably not, but any editor with code completion will load the modules.

dlech · 2017-10-03T16:05:04Z

Since python is a dynamic language, we could use that list (during import) to create properties corresponding to the buttons inside Button class.

(Warning, getting a bit off topic here)

👍 for taking advantage of dynamic nature of python to adapt to platforms, but I think I would rather see a single function that works for all buttons rather than identical functions/properties for each button. For example, I have a USB keypad that I can use for BrickPi/PiStorms that has the essential 6 EV3 buttons plus a bunch more. If I wanted to take advantage of other buttons, say the 10 numeric digits, then I would have to write 10 functions, one to handle each button. Whereas, if the button key code is just passed as a parameter, then I just have to write one function with a bunch of elifs.

dwalton76 · 2017-10-04T00:52:12Z

Working on LEDs now...

dwalton76 · 2017-10-04T03:08:42Z

LEDs are cleaned up, I tested them on EV3 and tried testing on pistorms but ran into ev3dev/ev3dev#978

I think this is far enough along to merge and then we can tackle anything else via separate issues. Will merge tomorrow unless folks want me to hold off.

dwalton76 · 2017-10-04T03:11:09Z

Also resolves #379

ddemidov · 2017-10-04T13:43:32Z

We could put a board-info file into ddemidov/ev3dev-lang-fake-sys

cool, @ddemidov can you add that? I haven't done much with ev3dev-lang-fake-sys

Done in ev3dev/ev3dev-lang-fake-sys@61d8ded

dwalton76 added 8 commits October 2, 2017 02:16

Fix Button(s)

8796a2a

Fix Button(s)

42fb03a

Fix motor(s)

27bc813

rm autogen-config.json

20bd353

remove bad imports

6bfa33a

Fix LEDs

95c8540

Move webserver.py to control/

7f3a41a

Fix tests/api_tests.py

3af4e35

rm helper.py

d2a2687

Unite _button and button

f775c1e

dlech reviewed Oct 3, 2017

View reviewed changes

dwalton76 added 3 commits October 3, 2017 21:37

Move PowerSupply from __init__.py to power.py

f7b5dc0

sensor.py input platform specific INPUT_1, etc

9c633bf

Uppercase buttons_filename and evdev_device_name constants

6f50f32

dwalton76 added 4 commits October 4, 2017 02:31

Unite _led.py and led.py

eb792c2

groundwork for pistorms LED support

cb76ea4

Do not barf if platform does not have LEDs

bc4ef56

raise MissingButton if platform does not have button

a1743c6

dwalton76 merged commit 5e33dff into ev3dev:develop Oct 4, 2017

dwalton76 deleted the file-musical-chairs branch October 4, 2017 11:32

dwalton76 mentioned this pull request Oct 4, 2017

Fix steering issue #394

Merged

Conversation

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WasabiFan commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Before

After

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

ddemidov commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

WasabiFan commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dlech commented Oct 3, 2017

Uh oh!

dlech Oct 3, 2017

Choose a reason for hiding this comment

Uh oh!

dwalton76 Oct 3, 2017

Choose a reason for hiding this comment

Uh oh!

dwalton76 commented Oct 3, 2017

Uh oh!

dlech Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

dwalton76 commented Oct 3, 2017 •

edited

Loading

dwalton76 commented Oct 3, 2017 •

edited

Loading

dlech commented Oct 3, 2017 •

edited

Loading

dlech Oct 3, 2017 •

edited

Loading